Fix/micro patches #811
Fix/micro patches #811
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Seems it broke windows builds. I'll figure it out tomorrow. |
|
Seems it has no patch file checking before applying patches. The directory separator caused the patch does not exist and failed to patch, but no exception. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how micro patches are applied by centralizing patch logic into the existing patchFile method and updating default patch lists in configuration.
- Removed legacy
FileSystem::addSourceExtractHook('micro', …)and simplifiedpatchMicrosignature. - Switched manual file concatenation in
patchMicroto reusepatchFile. - Updated
BuilderBaseto invokepatchMicroafter source extraction and trimmedSPC_MICRO_PATCHESdefaults inenv.ini.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/SPC/store/SourcePatcher.php | Simplified patchMicro, enhanced patchFile to handle absolute paths |
| src/SPC/builder/BuilderBase.php | Added invocation of SourcePatcher::patchMicro() after ext extraction |
| config/env.ini | Reduced default SPC_MICRO_PATCHES entries for Win32 and macOS contexts |
Comments suppressed due to low confidence (3)
src/SPC/store/SourcePatcher.php:107
- Update the docblock above
patchMicroto reflect its new signature (removing$nameand$target, and documenting the$itemsparameter correctly).
public static function patchMicro(?array $items = null): bool
src/SPC/store/SourcePatcher.php:176
- Inconsistent handling of missing patch files: sometimes returns
falseand later throws an exception. Consider standardizing on one approach for missing patches.
return false;
src/SPC/store/SourcePatcher.php:107
- [nitpick] The parameter name
$itemsis ambiguous; consider renaming it to something more descriptive like$patchesto clarify its purpose.
public static function patchMicro(?array $items = null): bool
| } else { | ||
| $patch_file = $patch_name; | ||
| } | ||
| if (!file_exists($patch_file)) { |
There was a problem hiding this comment.
[nitpick] The initial file_exists($patch_file) check makes the subsequent file_exists($patch_str) check redundant; consider removing one or consolidating the logic.
Copilot uses AI. Check for mistakes.
| if (\Phar::running() !== '') { | ||
| file_put_contents(SOURCE_PATH . '/' . $patch_name, file_get_contents($patch_file)); | ||
| $patch_str = str_replace('/', DIRECTORY_SEPARATOR, SOURCE_PATH . '/' . $patch_name); | ||
| if (str_starts_with($patch_str, 'phar://')) { |
There was a problem hiding this comment.
Checking str_starts_with($patch_str, 'phar://') may never detect the PHAR context correctly since $patch_str is a filesystem path. Consider using \Phar::running() to detect runtime in a PHAR archive as before.
| if (str_starts_with($patch_str, 'phar://')) { | |
| if (\Phar::running() !== '') { |
Copilot uses AI. Check for mistakes.
What does this PR do?
Fix #803 .
This PR will put micro patches after init souces and reuse
patchFilefunction to automatically skip the patched patches.Checklist before merging
*.phpor*.json, run them locally to ensure your changes are valid:PHP_CS_FIXER_IGNORE_ENV=1 composer cs-fixcomposer analysecomposer testbin/spc dev:sort-configsrc/globals/test-extensions.php.extension testortest extensionsto trigger full test suite.